please make it major since it has breaking changes

please remember to add full changeset and docs

saleor/saleor-docs#1437

won't it have the same names as fetch api? It would be confusing, probably better to also add alias on export

so next-app-router file can include export { FetchApi as NextAppRouterWebhook } from '../fetch-api' or something like this

would be easier if we see docs at this point

This is fine, since exported names do not reference Fetch API in any way, names are the same as in Next.js handlers except they are in a different folder. Our exports are:

• createAppRegisterHandler
• createManifestHandler
• createProtectedHandler
• SaleorSyncWebhook
• SaleorAsyncWebhook

what is "fetch middleware"? If this is a middleware using Fetch Request API, I don't think we should call it that way. Maybe just "middleware"?

probably you should avoid any in favor of unknown

please add tests to all middlewares

we are actually checking "saleorApiUrl", not domain, and it 1.0.0 we will drop domain checking

maybe can we extract this? Middleware checks header but also checks if auth data exists. And doesnt do anything with it. It doesnt event attach it to request, so it's cached

we can remove this, in v1 we can remove domain header, so no need to check this

Im not sure if middleware should work as "check" only. If you check against e.g. existence of AuthData, you can pass Request and middleware can enrich it with context - eg attach AuthData there

I don't think modifying Request object is a good idea. It would be like adding new properties to globals like Array or Window 😬 These objects are meant to be immutable.

Adding context is usually done by web frameworks that use Fetch API by creating new Context object and passing it everywhere, or by using AsyncLocalStorage. Example in Hono.

We could do this, but then we would need to have some context mechanism of our own. We could also use AsyncLocalStorage, but this is also problematic since runtimes like Cloudflare Workers, or Deno require enabling Node compatibility layer to do this :/

Modifying request isn't anything new. It's not about modifying global, but instance. It's not about Array.prototype.foo by (new Array()).foo

Context you mentioned is a platform context.
What I mean is request context. It can be achieved several ways, eg with async_hooks, modifying request or adding another argument (like createWebhookHandler(req, res, ctx)). Or how TRPC middleware works.

You can also read eg https://expressjs.com/en/guide/using-middleware.html

What I want to achieve is to allow middleware to attach data for next middlewares or handlers and compose the rich request in the end.

For example, if your handler fetches AuthData, it can attach it to request (pass to next step), instead of fetching it again in next middleware/handler

OK, I think we are talking about different things here and maybe the name middleware in this place is wrong and confusing 😅

These "middlewares" do not have any data we would want to pass as context, since they don't do any asynchronous actions.

This specific register action has a concept of "context" in its config, where you can attach to different events, e.g.:

  /**
   * Run right after Saleor calls this endpoint
   */
  onRequestStart?(
    request: RequestType,
    context: {
      authToken?: string;
      saleorDomain?: string;
      saleorApiUrl?: string;
      respondWithError: CallbackErrorHandler;
    }
  ): Promise<void>;

Config shape is the same as previous Next.js actions in app-sdk, it just has different Request object type depending on platform

createProtectedHandler also has its own context:

export type ProtectedHandlerContext = {
  baseUrl: string;
  authData: AuthData;
  user: TokenUserPayload;
};

Webhook handlers also have context (no change from previous public API):

export type SaleorWebhookHandler<TPayload = unknown, TExtras = {}> = (
  req: Request,
  ctx: WebhookContext<TPayload> & TExtras
) => Response | Promise<Response>;

As for fetch-middleware, please see my comment: #380 (comment).

comment is stale

why? the comment is still valid because we are not doing strict check on saleorApiUrl for compatibility reasons (I didn't change bussiness logic, rather I just copied our old code and adapted it to work with our new adapter abstraction).

We can make another breaking change and just drop the Saleor-Domain usage altogether imho